Skip to content

build: fix MSVC 2022 Release compilation - #46228

Closed
vmoroz wants to merge 3 commits into
nodejs:mainfrom
vmoroz:PR/Fix_MSVC_2022_Release_build
Closed

vmoroz wants to merge 3 commits into
nodejs:mainfrom
vmoroz:PR/Fix_MSVC_2022_Release_build

Conversation

@vmoroz

@vmoroz vmoroz commented Jan 16, 2023 •

Copy link
Copy Markdown
Member

This PR targets to address the Linker errors mentioned in Issue #43092 when we build Node.JS main branch in MSVC 2022 (Version 17.4.4).
Then, it fixes the VS2022 ARM64 build issue with file access violation originally fixed by PR #46231.

The fix adds PCH file to mksnapshot project in the v8.gyp file to solve #43092.
directory.build.props is added to override PreprocessedFileName output path to address the file access violation issue.

Details

When we build Node.JS Release in MSVC 2022 we see linker errors that mksnapshot cannot find FixedArray::get method. This is an example of the linker error from issue #43092:

platform-embedded-file-writer-aix.obj : error LNK2001: unresolved external symbol "public: class v8::internal::Object __cdecl v8::internal::FixedArray::get(int)const " (?get@FixedArray@internal@v8@@QEBA?AVObject@23@H@Z) [F:\node\tools\v8_gypfiles\mksna
pshot.vcxproj]

It seemed that the inlined method FixedArray::get is optimized out.
We can either change V8 files and explicitly include fixed-array-inl.h file as in PR #46231, or we can use the forced file inclusion used by PCH files. Adding the MSVC specific PCH for mksnapshot project addresses the issue.

The root cause of the access violation issue is that the embedded.S file is generated in the $(IntDir) folder while the preprocessed file name uses exactly the same path: <PreprocessedFileName Condition="'%(PreprocessedFileName)' == ''">$(IntDir)%(FileName)%(Extension)</PreprocessedFileName> from "c:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VC\v170\BuildCustomizations\marmasm.props". Thus, we are getting error because we try to change the embedded.S file while reading it.

In this PR we add new directory.build.props file that locally overrides the marmasm.props definition by adding .pp extension to the file name. It generates output file name that is different in from the input file name and addresses the file access violation issue.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants